Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use d3-dsv package for parsing tsv/csv files. #741

Merged
merged 3 commits into from
Apr 30, 2021

Conversation

adrianmroz-allegro
Copy link
Contributor

Closes #738

Around version 4, d3 changed it distribution model. Before it sued one package for all modules, later each module got it own package. We can't easily update d3 to version 4 (or later).

I added d3-dsv package. In theory I added package that duplicates functionality of d3 package. But webpack should not include d3 functions for handling csv/tsv to client code (due to tree shaking) and d3-dsv is only used on server (I even moved parser util from common to server).

…nction which converts values from tsv/csv file to javascript types.
@@ -197,6 +197,39 @@ dataCubes:
title: Records count
formula: $main.count()

- name: unemployment
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@mkuthan
Copy link
Member

mkuthan commented Apr 29, 2021

I added d3-dsv package. In theory I added package that duplicates functionality of d3 package. But webpack should not include d3 functions for handling csv/tsv to client code (due to tree shaking) and d3-dsv is only used on server (I even moved parser util from common to server).

Could you compare the size of the final bundle, please?

@adrianmroz-allegro
Copy link
Contributor Author

adrianmroz-allegro commented Apr 29, 2021

Could you compare the size of the final bundle, please?

No difference at all.

5940532 vs 5940532

@adrianmroz-allegro
Copy link
Contributor Author

@geohci did you have a chance to test it on your end?

@geohci
Copy link

geohci commented Apr 30, 2021

Thanks for the ping @adrianmroz-allegro -- pulled in the branch and worked perfectly for me (both TSV and CSV). Thanks!

@adrianmroz-allegro adrianmroz-allegro merged commit 24a8032 into master Apr 30, 2021
@adrianmroz-allegro adrianmroz-allegro deleted the bugfix/dsv-support branch April 30, 2021 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TSV/CSV native files won't recognize data because it's all loaded in as strings
3 participants